Skip to content

Initial implementation of proper TLS MAC handling#360

Open
ColtonWilley wants to merge 3 commits intomasterfrom
wp_tls_mac_pad_fix
Open

Initial implementation of proper TLS MAC handling#360
ColtonWilley wants to merge 3 commits intomasterfrom
wp_tls_mac_pad_fix

Conversation

@ColtonWilley
Copy link
Contributor

Missing handling for OSSL_CIPHER_PARAM_TLS_MAC leads to a mac verify failure when using CBC-MAC. Implement proper handling similar to openssl.

*/
static void wp_aes_block_freectx(wp_AesBlockCtx *ctx)
{
if (ctx->tlsmacAlloced) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use ->tlsmac != NULL instead, or does the tlsmacAlloced add something?

XMEMCPY(dst, src, sizeof(*src));
/* Deep-copy tlsmac to avoid double-free between src and dst. */
if (src->tlsmacAlloced && src->tlsmac != NULL) {
dst->tlsmac = OPENSSL_malloc(src->tlsmacsize);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should tlsmacAlloced be set following this allocation?

size_t good;
size_t i, j;
size_t macSize = ctx->tlsmacsize;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this is a CT function, but WOLFPROV_ENTER and WOLFPROV_EXIT could be helpful logs here (and are not dependent on crypto status)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants